-
Notifications
You must be signed in to change notification settings - Fork 550
Fix invokable objects incorrectly labeled as instances of Closure
#4797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 2.1.x
Are you sure you want to change the base?
Conversation
a02e586 to
6d1e13c
Compare
src/Type/Accessory/HasMethodType.php
Outdated
| if ( | ||
| $this->isCallable()->yes() | ||
| && $otherType->isCallable()->yes() | ||
| && !($otherType->isObject()->yes() && $otherType->isInstanceOf(Closure::class)->yes()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ondrejmirtes Hi, I'm not sure how to proceed here. I tried using instanceof, but PHPStan said I should use isObject() instead. Now I'm getting this error. Any advice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see other similar spots in the codebase.
it need to look like (new ObjectType(Closure::class))->isSuperTypeOf(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@staabm Thank you for the help, it seems ok now.
| $this->assertNoErrors($errors); | ||
| } | ||
|
|
||
| public function testBug13975(): void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your test needs to be moved into a *RuleTest which relates to the error message you no longer want to have.
AnalyzerIntegrationTest is reserved for performance and crash-tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess an additional test like https://phpstan.org/r/3cdd0efb-d240-49dd-9073-d1ada4d9843e might kill the mutant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems you lost the original test-case while adding the new one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, I misunderstood you, fixed
1a0435a to
47220f6
Compare
Closes phpstan/phpstan#13975